-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Unwrap call of function passed to move_only_function
#5808
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AlexGuteniev
commented
Oct 25, 2025
AlexGuteniev
commented
Oct 25, 2025
AlexGuteniev
commented
Oct 26, 2025
AlexGuteniev
commented
Oct 26, 2025
AlexGuteniev
commented
Oct 26, 2025
AlexGuteniev
commented
Oct 26, 2025
tests/std/tests/GH_005504_avoid_function_call_wrapping/test.cpp
Outdated
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
tests/std/tests/GH_005504_avoid_function_call_wrapping/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/GH_005504_avoid_function_call_wrapping/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/GH_005504_avoid_function_call_wrapping/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/GH_005504_avoid_function_call_wrapping/test.cpp
Outdated
Show resolved
Hide resolved
StephanTLavavej
approved these changes
Nov 11, 2025
This comment was marked as resolved.
This comment was marked as resolved.
Member
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
Member
📦 🎁 💝 |
This was referenced Nov 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Towards #5504.
One of the parts of the issue that can be addressed without having
copyable_function.✅ Coverage
Added just copy counter to see that the optimization works. It is copied zero times due to some core language optimization, but without the optimization it is copied once in the cases where it was missing.
Also added allocation counters to see that the other part of the optimization works.
Exercise different wrapping and also null functions.
⚙️ Optimization
Extract the inner
_Func_impl/_Func_impl_no_allocand use directly it without the containingfunction. This both gets call unwrapping and allocation avoidance.This change is only complicated in the current ABI. If in a future ABI it is possible to implement
functionthe same way thatmove_only_functionis implemented, things would be way easier.For 32-bit we can't easily avoid allocation of small function. The buffer of
functionis aligned tomax_align_tand the buffer insidemove_only_functionis aligned only asvoid*if it is next to vTable.move_only_functioncan contain objects ofmax_align_talignment, but in that case the buffer size would be smaller. As type erasure already happened by the time we havefunction, we can't query actual size or alignment.We probably could change
move_only_functionto make is emulated vtable the last pointer instead of the first, as we're not ABI-bound for/std:c++latestyet. But it doesn't look like a good idea, considering other, more commonmove_only_functionusage.❄️ Special cases
We throw
bad_function_call, and don't makemove_only_functioncontaining nullfunctionnull itself. Because the Standard is spelled out this way currently.If
functionis constructed insidemove_only_functionusingin_place_type, this optimization is not engaged.Also see #5806